Add codespell support (config, workflow to detect/not fix) and make it fix few typos #23398

yarikoptic · 2024-07-24T20:46:39Z

codespell is not new to this project but not really part of the automated workflow(s) (precommit, CI).

There is Makefile rule but had hardcoded settings into it, so people had to use make and could not pre-commit or directly -- I moved configuration into pyproject.yaml so now if you have codespell (and tomli) installed - you can just codespell.

CI workflow has 'permissions' set only to 'read' so also should be safe.

Also added pre-commit hook for it so no new typos would sneak in.

attn @rhatdan as I believe the codespell user ;)

openshift-ci · 2024-07-24T20:46:43Z

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Luap99

Thanks, I am fine with having codespell in CI but you properly want to wait for feedback from other maintainers first before spending more time on this in case others do not like it

Luap99 · 2024-07-25T08:54:21Z

.github/workflows/codespell.yml

We do not use github actions for the most part so please delete that.

If we want to enforce this on CI it must run in cirrus as part of make validate-sources, given you already hooked this into pre-commit this is already the case so it works

I will

delete GH action

add a TEMP commit with a typo to ensure they do get detected with pre-commit (will take PR into draft for the duration of TEMP commit presence).

Apparently both "Cirrus CI / Windows Cross" and "Cirrus CI / Validate source code changes" ran pre-commit and failed on codespell. As the point of test catching new typos is fulfilled, I will remove TEMP commit now and take out of draft.

Luap99 · 2024-07-25T08:55:56Z

pyproject.toml

@@ -13,3 +13,10 @@ exclude = '''
  | hack
 )/
 '''
+
+[tool.codespell]


I don't like that, it feels totally arbitrary to put this unrelated configuration in a python config file.

I recommend to use .codespellrc

Up to you. I am even a little on your side since then it would not require optional tomli.

Just 1 more chance to reconsider under the following arguments

often people prefer avoiding pollution of root directory with per-tool configuration files.

you already have the one (and only content) tool.black in pyproject.toml

FWIW codespell is written in Python , if argument "black is written in Python" comes up ;-)

So let me know

I don't want this in a random python file, we are not a python project so please remove it from there. I don't think anyone will ever look into this file ever and fine the codespell config there.

rhatdan · 2024-07-25T09:03:49Z

I like this alot, thanks @yarikoptic. Fix the couple of issues @Luap99 found, and I will give it a LGTM

Luap99 · 2024-07-25T09:07:50Z

Also if you repush can you please fix up the commit messages for the [DATALAD RUNCMD] commits, that is completely useless data for any human reader of the commit messages

rhatdan · 2024-07-25T09:22:40Z

I would be fine with just merging the commits into one PR.

yarikoptic · 2024-07-25T21:09:03Z

Also if you repush can you please fix up the commit messages for the [DATALAD RUNCMD] commits, that is completely useless data for any human reader of the commit messages

FWIW: I can easily adjust short commit message and can remove internal record on how the modification was made (some humans are curious creatures and actually do like to see command line invocation which resulted in the committed diff... More on datalad run at https://handbook.datalad.org/en/latest/basics/basics-run.html ... It also becomes "fun" to do "functional rebase" whenever commit is taken not as a diff, but rather as "do this action" (like codespell -w or some sed -i invocation), thus avoiding dealing with conflicts or simply missing some new changes to be done.

With that in mind, just tell me final verdict of

C1: leave as is
C2: adjust short summary
C3: adjust full commit message and remove embedded json record with the command

rhatdan · 2024-08-02T07:49:42Z

LGTM
@Luap99 PTAL

Luap99 · 2024-08-02T08:33:17Z

C3: adjust full commit message and remove embedded json record with the command

I am reading commit message and this random stuff just wastes every readers time trying to make sense of this and why it is there. Please just squash the codespell fixes in a commit and name it something like fix issues reported by codespell or something like this.

Luap99 · 2024-08-02T08:36:04Z

Makefile

@@ -309,7 +309,7 @@ test/version/version: version/version.go

 .PHONY: codespell
 codespell:
-	codespell -S bin,vendor,.git,go.sum,.cirrus.yml,"*.fish,RELEASE_NOTES.md,*.xz,*.gz,*.ps1,*.tar,swagger.yaml,*.tgz,bin2img,*ico,*.png,*.1,*.5,copyimg,*.orig,apidoc.go" -L ddress,secon,passt,bu,hastable,te,clos,ans,pullrequest,uint,iff,od,seeked,splitted,marge,erro,hist,ether,specif -w
+	codespell -w


it would help to add a comment here saying where the new config is for authors used to edit the rules here inline.

added a comment

yarikoptic · 2024-08-05T16:27:39Z

moved config into new .codespellrc
rebased and reran fixing (there was one new typo)
squashed multiple commits and minimized their commit messages - now only 3 commits. Feel welcome to squash into a single one upon merging

github-actions · 2024-09-05T00:08:07Z

A friendly reminder that this PR had no activity for 30 days.

rhatdan · 2024-09-05T10:06:33Z

@yarikoptic still working on this? Looks like all it needs is to adjust the commit message.

…kefile Signed-off-by: Yaroslav Halchenko <[email protected]>

Signed-off-by: Yaroslav Halchenko <[email protected]>

yarikoptic · 2024-09-05T17:57:37Z

Well, I thought I was done with it! ;) now spotted that failed CI apparently complaining about overly long subject line!

 * 9bde6b551 "Add codespell config, pre-commit definition, and move used options from Makefile into config" ... FAIL
  - FAIL - commit subject exceeds 90 characters

ok,

reworded
Since that rewrote history -- rebased again
saw that 2 new typos detected, made it fix those and fixup that last commit with other fixes
force pushed

rhatdan · 2024-09-05T18:34:29Z

/approve
/lgtm

openshift-ci · 2024-09-05T18:34:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, yarikoptic

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rhatdan · 2024-09-05T18:34:46Z

Thanks @yarikoptic

openshift-ci bot added the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label Jul 24, 2024

rhatdan added No New Tests Allow PR to proceed without adding regression tests and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Jul 25, 2024

Luap99 requested changes Jul 25, 2024

View reviewed changes

yarikoptic force-pushed the enh-codespell branch from 0cca676 to acf2328 Compare July 25, 2024 21:11

yarikoptic marked this pull request as draft July 25, 2024 21:12

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 25, 2024

yarikoptic force-pushed the enh-codespell branch from acf2328 to bee2654 Compare July 25, 2024 21:25

yarikoptic marked this pull request as ready for review July 25, 2024 21:25

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 25, 2024

yarikoptic requested a review from Luap99 July 25, 2024 21:25

edsantiago mentioned this pull request Aug 1, 2024

podman-machine uncategorized flakes #22551

Open

Luap99 reviewed Aug 2, 2024

View reviewed changes

yarikoptic force-pushed the enh-codespell branch from bee2654 to b09f38d Compare August 5, 2024 16:25

github-actions bot added the stale-pr label Sep 5, 2024

yarikoptic added 3 commits September 5, 2024 13:56

Add codespell config, pre-commit definition, and move options from Ma…

4700b5a

…kefile Signed-off-by: Yaroslav Halchenko <[email protected]>

Fix typo: replace buildin with built-in

12b3d01

Signed-off-by: Yaroslav Halchenko <[email protected]>

Detect and fix typos using codespell

c203c48

Signed-off-by: Yaroslav Halchenko <[email protected]>

yarikoptic force-pushed the enh-codespell branch from b09f38d to c203c48 Compare September 5, 2024 17:57

rhatdan removed the stale-pr label Sep 5, 2024

openshift-ci bot assigned rhatdan Sep 5, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 5, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 5, 2024

openshift-merge-bot bot merged commit ae14dff into containers:main Sep 5, 2024
87 of 88 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add codespell support (config, workflow to detect/not fix) and make it fix few typos #23398

Add codespell support (config, workflow to detect/not fix) and make it fix few typos #23398

yarikoptic commented Jul 24, 2024

openshift-ci bot commented Jul 24, 2024

Luap99 left a comment

Luap99 Jul 25, 2024

yarikoptic Jul 25, 2024

yarikoptic Jul 25, 2024

Luap99 Jul 25, 2024

yarikoptic Jul 25, 2024

Luap99 Aug 2, 2024

rhatdan commented Jul 25, 2024

Luap99 commented Jul 25, 2024

rhatdan commented Jul 25, 2024

yarikoptic commented Jul 25, 2024 •

edited

Loading

rhatdan commented Aug 2, 2024

Luap99 commented Aug 2, 2024

Luap99 Aug 2, 2024

yarikoptic Aug 5, 2024

yarikoptic commented Aug 5, 2024

github-actions bot commented Sep 5, 2024

rhatdan commented Sep 5, 2024

yarikoptic commented Sep 5, 2024

rhatdan commented Sep 5, 2024

openshift-ci bot commented Sep 5, 2024

rhatdan commented Sep 5, 2024

Add codespell support (config, workflow to detect/not fix) and make it fix few typos #23398

Add codespell support (config, workflow to detect/not fix) and make it fix few typos #23398

Conversation

yarikoptic commented Jul 24, 2024

openshift-ci bot commented Jul 24, 2024

Luap99 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhatdan commented Jul 25, 2024

Luap99 commented Jul 25, 2024

rhatdan commented Jul 25, 2024

yarikoptic commented Jul 25, 2024 • edited Loading

rhatdan commented Aug 2, 2024

Luap99 commented Aug 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yarikoptic commented Aug 5, 2024

github-actions bot commented Sep 5, 2024

rhatdan commented Sep 5, 2024

yarikoptic commented Sep 5, 2024

rhatdan commented Sep 5, 2024

openshift-ci bot commented Sep 5, 2024

rhatdan commented Sep 5, 2024

yarikoptic commented Jul 25, 2024 •

edited

Loading